Add an ActiveMQ Artemis extension #2930

middagj · 2019-06-21T22:10:45Z

I have added an extension to support ActiveMQ Artemis via Core or JMS. Using AMQP via reactive messaging does not suit our needs as we want to control the acknowledgements and commits.

I have kept the CDI part small and only produce a ServerLocater (Core) and ActiveMQJMSConnectionFactory (JMS). These components should not be created again even if the connection dies for some reason. I don't know if building a framework around this (using an annotation for the listener method, create a proper producing template with serialization and transactions) is needed. It would take considerably more time, but would definitely add developer joy.

I also have chosen to support a single url property and not all the options separately. In Artemis you can specify the options there (like JDBC).

The changes for the Netty native libraries I have put in the NettyProcessor. They postpone initialization of those classes to runtime. I have disable epoll and kqueue in SubstrateVM for Artemis because of an UnsatisfiedLinkError. Others also seem to fail, e.g. Ratpack, Vert.x.

middagj · 2019-06-22T08:24:53Z

The job running on agent Hosted Agent has exceeded the maximum execution time of 150.

Is this normal? A quick inspection in the log shows that Maven downloads at very low transfer rates.

gsmet · 2019-06-22T10:16:38Z

@middagj don't worry about it, we need to set the timeout higher.

geoand · 2019-06-22T13:00:28Z

I already uped the timeout as part of #2910

geoand · 2019-06-22T15:28:52Z

@middagj could you please rebase onto the latest master to fix the conflicts?

gastaldi · 2019-06-22T15:48:32Z

I wonder if this PR solves #1843?

middagj · 2019-06-22T19:24:39Z

I have rebased on master. Yes this would solve #1843, but you should still manually enable JNI.

geoand · 2019-06-22T19:34:18Z

@gsmet perhaps we need to add a BuildItem that enables JNI?

cescoffier · 2019-06-23T17:47:01Z

@middagj Just to clarify, Reactive Messaging gives you total control on the acknowledgment using Message.ack():

CompletionStage<Void> consume(Message<String> message) {
   // do something with the message
  // acknoweldge
  return message.ack();
  // you will get the next message once the previous is acknowledged (back pressure)
}

In addition, did you look at the impact on the native executable size of the runtime initialization items?

middagj · 2019-06-23T19:14:33Z

Thanks, didn't know that was possible. However, I don't see transactions being possible in the reactive messaging framework, which we would like to use.

I did not look at the impact on the native executable size. Just did a comparison by disabling the runtime initializations and excluding the epoll and kqueue libraries explicitly (needed the --allow-incomplete-classpath flag) vs the proposed solution . On a small working example the difference is around 340kB on 58MB. But this will disable epoll and kqueue also when not running native but in the VM and gives NoClassDefFoundError exceptions when running.

emmanuelbernard · 2019-06-24T13:11:06Z

@cescoffier do you mind if I make you captain for that PR review. You're probably the one with the best view on how we want to proceed here.

middagj · 2019-06-24T13:17:56Z

I don't know if the label reactive messaging really fits this PR. My goal of this PR is to add support for the Artemis Core/JMS client in Quarkus. One could already use Artemis with the reactive messaging framework via AMQP, as already demonstrated in the AMQP guide.

emmanuelbernard · 2019-06-24T13:18:10Z

integration-tests/artemis-core/src/main/java/io/quarkus/it/artemis/ArtemisProducerManager.java

+
+    @PostConstruct
+    public void init() throws Exception {
+        connection = serverLocator.createSessionFactory();


I'm not quite sure whyw e don't enlist a CDI bean being the session factory provider in the extension

Because a ClientSessionFactory should be regarded as connection. See Artemis documentation. If your connection breaks, the ClientSessionFactory should be created again.

@middagj OK but that feels like a terrible developer experience.
The extension should make an effort to retry this connection itself. You can imagine very few users would think of doing it otherwise. Any advice on how to do it with CDI @mkouba

Well, if I understand it correctly we could create bean that implements ClientSessionFactory and delegates to a factory created by serverLocator.createSessionFactory(). Then in every call we could verify the underlying factory and if something is wrong, recreate the factory again? @middagj @emmanuelbernard But I don't know anything about Artemis. So this is just a guess.

I think the developer joy can be greatly improved. I would like to not have to worry about a connection at all and just want to define a message listener and have a producer bean which can be used to send new messages in the same transaction if configured. I don't know when all that would realistic to add on, and having the ability to use Artemis client in Quarkus already helps us and probably others already.

Sounds like a good solution. There are different session factories which can be created, but that can be put in a config.

I was trying this approach, but I reconsider if this is the right way. ClientSessionFactory implements AutoCloseable and it resembles the normal JMS Connection class. If you look at both the JMS and Core ArtemisProducerManager, they look quite similar. Both protocols have the distinction between a Connection and a Session, only Core protocol names the Connection a SessionFactory.

I think it would be better to focus on JMS and create something similar like Spring's JmsListener and JmsTemplate to improve developer joy. I prefer to do it in a separate pull request as this request already enables people to add the dependency in a native image.

...loyment/src/main/java/io/quarkus/infinispan/client/deployment/InfinispanClientProcessor.java

...s/artemis-core/runtime/src/main/java/io/quarkus/artemis/core/runtime/ArtemisBuildConfig.java

emmanuelbernard · 2019-06-24T16:08:34Z

Yes don't worry too much about the label name here.

emmanuelbernard · 2019-06-25T09:37:14Z

That's my understanding too Martin

…

On Tue, Jun 25, 2019 at 9:08 AM Martin Kouba ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In integration-tests/artemis-core/src/main/java/io/quarkus/it/artemis/ArtemisProducerManager.java <#2930 (comment)>: > +import org.apache.activemq.artemis.api.core.client.ClientMessage; +import org.apache.activemq.artemis.api.core.client.ClientSession; +import org.apache.activemq.artemis.api.core.client.ClientSessionFactory; +import org.apache.activemq.artemis.api.core.client.ServerLocator; + ***@***.*** +public class ArtemisProducerManager { + + @Inject + ServerLocator serverLocator; + + private ClientSessionFactory connection; + + @PostConstruct + public void init() throws Exception { + connection = serverLocator.createSessionFactory(); Well, if I understand it correctly we could create bean that implements ClientSessionFactory and delegates to a factory created by serverLocator.createSessionFactory(). Then in every call we could verify the underlying factory and if something is wrong, recreate the factory again? @middagj <https://github.com/middagj> @emmanuelbernard <https://github.com/emmanuelbernard> But I don't know anything about Artemis. So this is just a guess. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2930?email_source=notifications&email_token=AACJNWFPFKUCORPUMQB5JQ3P4G76BA5CNFSM4H2VER42YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB4QMUXQ#discussion_r297035752>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACJNWD3YBGBWOF6S7RDXI3P4G76BANCNFSM4H2VER4Q> .

cescoffier

I've made one small suggestion.

The rest looks good. I would like to have the opinion of @dmlloyd and @stuartwdouglas on the Class.forName and the runtime initialization.

Side notes:

As you are exposing the ConnectionFactory, I can now implement the JMS support in reactive messaging which will use the produced ConnectionFactory
About transaction, it's coming, but far from being perfect as XA is a blocking protocol. Right now the solution is to off-load to a thread pool. But nevertheless, the JMS connector need a ConnectionFactory and I can use yours.

...ons/artemis-jms/runtime/src/main/java/io/quarkus/artemis/jms/runtime/ArtemisJmsProducer.java

middagj · 2019-06-25T14:10:27Z

If I have time today I will see if I can give an alternative for the Class.forName as suggested by @emmanuelbernard.

Perhaps it is another topic, but XA transactions aren't necessary to use JMS transactions.

dmlloyd · 2019-06-25T14:15:02Z

Every single messaging and database API having its own transaction API is not a feature ;)

/cc @mmusgrov et al

emmanuelbernard · 2019-06-25T14:30:16Z

On Tue, Jun 25, 2019 at 4:15 PM David M. Lloyd ***@***.***> wrote: Every single messaging and database API having its own transaction API is not a feature ;) /cc @mmusgrov <https://github.com/mmusgrov> et al

Why do you say that? Or rather to which part of that PR or discussion are you referring to?

dmlloyd · 2019-06-25T15:10:45Z

On Tue, Jun 25, 2019 at 4:15 PM David M. Lloyd @.***> wrote: Every single messaging and database API having its own transaction API is not a feature ;) /cc @mmusgrov https://github.com/mmusgrov et al
Why do you say that? Or rather to which part of that PR or discussion are you referring to?

I've just shamelessly co-opted the discussion to support a point in a completely separate conversation ;)

dmlloyd · 2019-07-10T19:47:16Z

This change now has conflicts. Could you please rebase?

middagj · 2019-07-11T21:41:15Z

I have fixed the conflicts and also fixed the requested changes by @cescoffier, any things that should be added for this pull request to be merged?

gsmet

Added a comment about the availability of JniBuildItem.

integration-tests/artemis-core/pom.xml

middagj · 2019-07-31T10:56:21Z

@gsmet I addressed your requested changes (and rebased again because of conflicts). Any other stuff that is blocking this pull request?

gsmet · 2019-08-19T11:06:28Z

@cescoffier could you drive this puppy home?

We need a rebase but, apart from that, could you check if it's ready to go in?

cescoffier · 2019-08-19T14:01:08Z

@gsmet The rest was ok to be merged.

middagj · 2019-09-16T12:21:00Z

Did I forgot to add something that should result in the extension to be available in Maven central? I cannot find it after the release of 0.22.0.

gsmet · 2019-09-16T12:31:48Z

0.22.0 is 0.21.2 + the Spring extensions + bug fixes

This will be released as part of 0.23.0, which we hope to release soon.

middagj · 2019-09-16T12:32:54Z

Thanks!

gastaldi · 2019-09-24T15:28:28Z

@middagj I couldn't find any documentation regarding this extension, could you add some before 0.23.0 is released (tomorrow) ? Thanks :)

middagj · 2019-09-24T18:03:38Z

Yes, it was on my todo list. I will add some today.

geoand added the triage/needs-rebase This PR needs to be rebased first because it has merge conflicts label Jun 22, 2019

emmanuelbernard added area/reactive-messaging and removed triage/needs-rebase This PR needs to be rebased first because it has merge conflicts labels Jun 24, 2019

emmanuelbernard reviewed Jun 24, 2019

View reviewed changes

cescoffier requested changes Jun 25, 2019

View reviewed changes

...ons/artemis-jms/runtime/src/main/java/io/quarkus/artemis/jms/runtime/ArtemisJmsProducer.java Outdated Show resolved Hide resolved

gsmet requested changes Jul 17, 2019

View reviewed changes

integration-tests/artemis-core/pom.xml Outdated Show resolved Hide resolved

gsmet assigned cescoffier Jul 19, 2019

cescoffier self-requested a review July 19, 2019 09:57

cescoffier approved these changes Jul 19, 2019

View reviewed changes

middagj mentioned this pull request Jul 19, 2019

Revert JniBuildItem in Netty and add in Amazon DynamoDB #3286

Merged

gsmet approved these changes Jul 31, 2019

View reviewed changes

middagj added 6 commits August 21, 2019 14:33

Add an ActiveMQ Artemis extension

f85934e

Use BuildItem to indicate if JMS should be used instead of Core

9c12805

Produce JMS ConnectionFactory instead of specific ActiveMQ

d6d8eef

Use JniBuildItem to enable JNI

9b61f85

Use Recorder instead of Template

9a95f3c

Set up a higher timeout for native builds

eb0a2a8

cescoffier merged commit 4f9cbc6 into quarkusio:master Aug 22, 2019

cescoffier added this to the 0.22.0 milestone Aug 22, 2019

cescoffier added the release/noteworthy-feature label Aug 22, 2019

middagj mentioned this pull request Sep 24, 2019

Add Artemis JMS documentation #4186

Merged

gsmet removed the area/reactive-messaging label Sep 25, 2019

Add an ActiveMQ Artemis extension #2930

Add an ActiveMQ Artemis extension #2930

Conversation

middagj commented Jun 21, 2019

middagj commented Jun 22, 2019

gsmet commented Jun 22, 2019

geoand commented Jun 22, 2019

geoand commented Jun 22, 2019

gastaldi commented Jun 22, 2019

middagj commented Jun 22, 2019 • edited Loading

geoand commented Jun 22, 2019

cescoffier commented Jun 23, 2019 • edited Loading

middagj commented Jun 23, 2019

emmanuelbernard commented Jun 24, 2019

middagj commented Jun 24, 2019

emmanuelbernard Jun 24, 2019

Choose a reason for hiding this comment

middagj Jun 24, 2019

Choose a reason for hiding this comment

emmanuelbernard Jun 24, 2019

Choose a reason for hiding this comment

mkouba Jun 25, 2019

Choose a reason for hiding this comment

middagj Jun 25, 2019

Choose a reason for hiding this comment

middagj Aug 17, 2019

Choose a reason for hiding this comment

emmanuelbernard commented Jun 24, 2019

emmanuelbernard commented Jun 25, 2019 via email

cescoffier left a comment

Choose a reason for hiding this comment

middagj commented Jun 25, 2019

dmlloyd commented Jun 25, 2019

emmanuelbernard commented Jun 25, 2019 via email

dmlloyd commented Jun 25, 2019

dmlloyd commented Jul 10, 2019

middagj commented Jul 11, 2019

gsmet left a comment

Choose a reason for hiding this comment

middagj commented Jul 31, 2019

gsmet commented Aug 19, 2019

cescoffier commented Aug 19, 2019

middagj commented Sep 16, 2019

gsmet commented Sep 16, 2019

middagj commented Sep 16, 2019

gastaldi commented Sep 24, 2019

middagj commented Sep 24, 2019

middagj commented Jun 22, 2019 •

edited

Loading

cescoffier commented Jun 23, 2019 •

edited

Loading